Skip to content

fix(controlplane): enforce project-scoped RBAC on AttestationService/GetContract - #3401

Merged
migmartri merged 1 commit into
chainloop-dev:mainfrom
migmartri:miguel/pfm-6717-cross-project-contractworkflow-disclosure-via
Sep 4, 2026
Merged

fix(controlplane): enforce project-scoped RBAC on AttestationService/GetContract#3401
migmartri merged 1 commit into
chainloop-dev:mainfrom
migmartri:miguel/pfm-6717-cross-project-contractworkflow-disclosure-via

Conversation

@migmartri

@migmartri migmartri commented Sep 4, 2026

Copy link
Copy Markdown
Member

A project-scoped API token could call AttestationService/GetContract with another project's name and receive that project's workflow metadata and full contract schema. Attestation endpoints are authenticated through the attestation JWT path and skipped by the authz middleware, so the handler is the only authorization point, and GetContract was the lone attestation handler that never checked the caller's rights on the requested project.

This adds the same project authorization the sibling Init, Store and Cancel handlers already perform, applied after the workflow is resolved so the project reference is validated first. A project-scoped token now only sees workflows and contracts in its own project, while org-scoped tokens, workflow-scoped tokens, user, legacy robot account and federated callers keep their existing behavior.

To let the service-layer integration tests reproduce the context the attestation middlewares produce, usercontext.WithRobotAccount is exported alongside the existing CurrentRobotAccount reader.

Fixes PFM-6717

🤖 Posted by Maximus bot (Claude Code) on behalf of @migmartri

Review in cubic

…GetContract

AttestationService/GetContract resolved the workflow org-scoped without
checking the caller's rights on the requested project, letting a
project-scoped API token read another project's workflow metadata and
full contract schema through the attestation JWT path, which the authz
middleware skips.

Apply the same project authorization the sibling Init/Store/Cancel
handlers already perform, via userHasPermissionOnProject with
PolicyWorkflowRead, after the workflow is resolved.

Export usercontext.WithRobotAccount so the service-layer integration
tests can build the same context the attestation middlewares produce.

Assisted-by: OpenCode
Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev>

Chainloop-Trace-Sessions: ses_f92bd9943ffeCzB41EHYLejTZR
@chainloop-platform

chainloop-platform Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

AI Session Checks — 🟡 74% · ✅ 0 failing

Avg score Sessions Failing policies Attribution Files Lines Total Duration
🟡 74% 1 ✅ 0 100% AI / 0% Human 6 +191 / -5 13m26s

🟡 74% — 100% AI — ✅ All policies passing

Sep 4, 2026 16:31 UTC · 13m26s · $2.36 · 177.1k in / 14.1k out · opencode 1.17.20 (z-ai/glm-5.3)

View session details ↗

Change Summary

  • Adds a project-permission check to AttestationService/GetContract for project-scoped callers.
  • Adds an integration regression test covering blocked and allowed project, workflow, and org token paths.
  • Exports usercontext.WithRobotAccount and updates middleware call sites so the service test can build matching auth context.

AI Session Overall Score

🟡 74% — Strong fix arc, but missing command results leave verification and some completion claims unconfirmed.

AI Session Analysis Breakdown

🟢 90% · user-trust-signal

No notes.

🟢 88% · scope-discipline

🟢 Touched files cluster around attestation and usercontext, plus one supporting test. · High Impact

🟢 85% · solution-quality

🟢 The fix pairs a new regression test with the existing authorization helper. · High Impact

🟡 72% · alignment

🟠 The assistant claimed reviews, signing, and PR readiness without any captured command outcomes. · Medium Severity

💡 When commands matter, quote or paste the observed result before summarizing them as complete.

🟡 70% · context-and-planning

🟢 AI built a concrete todo list before the first code edit. · High Impact

🟠 The opening ask was just a handoff filename for a multi-file security fix. · Medium Severity

💡 Front-load the ticket, constraints, and done criteria when the task spans multiple files.

🟡 55% · verification

🔴 New regression tests were written, but every test, build, and lint command is missing its captured result. · High Severity

💡 Capture the runner output or quote the pass line before claiming the suite is green.


File Attribution

████████████████████ 100% AI / 0% Human

Status Attribution File Lines
created ai app/controlplane/internal/service/attestation_integration_test.go +180 / -0
modified ai app/controlplane/internal/service/attestation.go +5 / -0
modified ai app/controlplane/internal/usercontext/robotaccount_middleware.go +3 / -2
modified ai app/controlplane/internal/usercontext/apitoken_middleware.go +1 / -1
modified ai app/controlplane/internal/usercontext/currentuser_middleware.go +1 / -1
modified ai app/controlplane/internal/usercontext/federated_middleware.go +1 / -1

Policies (4)

Status Policy Material Messages
✅ Passed ai-config-ai-agents-allowed ai-coding-session-ses-f9 -
✅ Passed ai-config-no-dangerous-commands ai-coding-session-ses-f9 -
✅ Passed ai-config-no-secrets ai-coding-session-ses-f9 -
✅ Passed ai-config-mcp-servers-allowed ai-coding-session-ses-f9 -

Security Checks — ✅ 7 passing

secret-scan

Status Policy Messages
✅ Passed secrets-detection -

sast-scan

Status Policy Messages
✅ Passed owasp-top10-2025 -
✅ Passed sast -
✅ Passed cwe-top25 -
✅ Passed cwe-top26-40-cusp -

PR info

Status Policy Messages
✅ Passed pr-description-required -
✅ Passed pr-user-story-linked -

⏭️ 3 scans not applied

Scan Reason
vulnerability-scan no manifest/lockfile changed
github-actions-scan no workflow files changed
iac-scan no IaC files changed

View attestation ↗


Powered by Chainloop and Chainloop Trace

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 6 files

Heads up: you’re close to your included review allowance. Set a flex budget so reviews don’t pause.

Re-trigger cubic

@migmartri
migmartri requested a review from a team September 4, 2026 16:47
@migmartri
migmartri merged commit e2828c6 into chainloop-dev:main Sep 4, 2026
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants